Guard PDRange constructors against invalid ranges#176
Closed
pingpingy1 wants to merge 1 commit into
Closed
Conversation
Author
|
Oops, didn't realize I hadn't written tests for the new guards. My bad. |
PDRange has three constructors, two of which takes a `COSArray range` input to describe the range, and one that also takes an index into the array. This commit adds additional guards for when 1. the input array is `null`, or 2. the index does not properly point to two elements of the array. The first case is handled by setting to the default range 0..1. The second throws an `IllegalArgumentException`.
Contributor
|
I'm not sure if this is helpful, because bad parameters may not just come from users, but from bad PDFs (although unlikely). A user constructing PDRange with a null COSArray will just get an NPE and IMHO this is fine. What remains is the case where a PDF has an array that is too short. It's possible that we do already catch this, for example |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR intends to provide suitable guards against invalid arguments to the
PDRangeconstructors.The first case is when the
COSArray rangeparameter is given thenullvalue.Here, the
nullvalue is directly saved to therangeArrayvariable, and cannot be set to a non-null value using any other public method.Hence, I argue that this case should be guarded against in the constructor, either by throwing an exception (for example, using the
Objects.requireNonNullmethod) or by setting the range to a default range (as implemented in my commit using 0..1).The second case is when the
indexparameter leads to invalid indices for the range's minimum and maximum values.Again, the provided
index's value is directly saved to thestartingIndexvariable, and cannot by changed via any public method.Moreover, no public method could lengthen the
rangeArray.The methods
setMinandsetMaxdo not check the validity of the indices2*startingIndexand2*startingIndex+1, which could throw exceptions down the line.Thus, I argue that this case also should be handled here, either by throwing an exception (as implemented in my commit using
IllegalArgumentException) or by using a default index (such as 0).